Skip to content

[netcdf-c] Update to 4.8.1, revise features#21239

Merged
vicroms merged 18 commits intomicrosoft:masterfrom
dg0yt:netcdf-c
Nov 19, 2021
Merged

[netcdf-c] Update to 4.8.1, revise features#21239
vicroms merged 18 commits intomicrosoft:masterfrom
dg0yt:netcdf-c

Conversation

@dg0yt
Copy link
Copy Markdown
Contributor

@dg0yt dg0yt commented Nov 7, 2021 •
edited
Loading

  • What does your PR fix?

    Updates netCDF-C.
    Decouples netcdf-4 feature from hdf5. (Hdf5 implies netcdf-4, but not vice versa.)
    Fixes hdf5 szip usage with static linkage (assuming it was broken already in previous versions).
    Exposes features for the new NCZarr support. (Needs for [aws-sdk-cpp] Fix the path in the export cmake config #21240 for building, but this should not hold back this PR.)
    Enables CI build with reduced feature set for uwp and arm64-windows.
    Enables using reduced set of dependencies (via explicit feature selection in manifest or with vcpkg install).

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    unchanged, no

  • Does your PR follow the maintainer guide?

    yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    yes

@dg0yt dg0yt mentioned this pull request Nov 7, 2021
Merged
@dg0yt dg0yt marked this pull request as draft November 7, 2021 14:26
@dg0yt dg0yt marked this pull request as ready for review November 7, 2021 19:38
@dg0yt dg0yt marked this pull request as draft November 7, 2021 19:43
@dg0yt dg0yt marked this pull request as ready for review November 7, 2021 19:51
@dg0yt
Copy link
Copy Markdown
Contributor Author

dg0yt commented Nov 10, 2021

[226/233] .. /debug ... -LIBPATH:G:\21239\vcpkg\installed\x64-windows-static\lib liblib\netcdf.lib ... G:\21239\vcpkg\installed\x64-windows-static\debug\lib\libcurl-d.lib G:\21239\vcpkg\installed\x64-windows-static\lib\zip.lib G:\21239\vcpkg\installed\x64-windows-static\debug\lib\libhdf5_D.lib ... G:\21239\vcpkg\installed\x64-windows-static\lib\libszip.lib ...

... and here is a strange mix of debug and release 💥 We need to check the imported configurations.

@dg0yt dg0yt marked this pull request as draft November 13, 2021 17:52
@dg0yt
Copy link
Copy Markdown
Contributor Author

dg0yt commented Nov 14, 2021

here is a strange mix of debug and release

This configuration comes from installed/x64-linux/share/aws-cpp-sdk-core/aws-cpp-sdk-core-targets.cmake:

INTERFACE_LINK_LIBRARIES "\$<LINK_ONLY:pthread>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/libcurl.a>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/libcurl-d.a>;\$<LINK_ONLY:dl>;\$<LINK_ONLY:-lpthread>;\$<LINK_ONLY:OpenSSL::SSL>;\$<LINK_ONLY:OpenSSL::Crypto>;\$<LINK_ONLY:ZLIB::ZLIB>;${_IMPORT_PREFIX}/lib/libssl.a;${_IMPORT_PREFIX}/lib/libcrypto.a;\$<LINK_ONLY:-lpthread>;\$<LINK_ONLY:dl>;\$<LINK_ONLY:dl>;\$<LINK_ONLY:-lpthread>;${_IMPORT_PREFIX}/lib/libz.a;AWS::aws-crt-cpp"

Related: #20289 (comment)

@dg0yt dg0yt mentioned this pull request Nov 14, 2021
Draft
@dg0yt
Copy link
Copy Markdown
Contributor Author

dg0yt commented Nov 15, 2021

aws-cpp-sdk is a difficult citizen in vcpkg. It is not ready for vcpkg-style debug+release. It exports dependencies incompletely (missing: s2n, stdc++) and not in the right order for static linkage.

@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Nov 15, 2021

@dg0yt Can you please report that to https://github.com/aws/aws-sdk-cpp?

@dg0yt
Copy link
Copy Markdown
Contributor Author

dg0yt commented Nov 15, 2021

@dg0yt Can you please report that to https://github.com/aws/aws-sdk-cpp?

I think we need to make port aws-sdk-cpp etc. up-to-date first.

I would suggest to remove feature nczarr-s3 from this port-version of netcdf-c. It needs more work in vcpkg and in upstreams.
I can keep the current state in a separate branch of my vcpkg fork.

@ahmedyarub ahmedyarub mentioned this pull request Nov 15, 2021
Merged
@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Nov 16, 2021

I will restart x64-osx test after the other tests finished.

@dg0yt dg0yt marked this pull request as ready for review November 16, 2021 07:50
@dg0yt
Copy link
Copy Markdown
Contributor Author

dg0yt commented Nov 16, 2021

You may re-run CI now. x64-linux is blocked for no obvious reason.

@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Nov 16, 2021

It seems that the feature changes fails to run.

@dg0yt
Copy link
Copy Markdown
Contributor Author

dg0yt commented Nov 17, 2021

Strange. The last commit didn't change a bit of port netcdf-c, and the debug run of vcpkg ci wasn`t used, even on osx. Reverting the last commit (as it is unrelated).

@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Nov 18, 2021

All features are tested successfully on x86-windows and x64-windows-static.

@JackBoosY JackBoosY added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed requires:all-feature-testing labels Nov 18, 2021
@vicroms vicroms merged commit 297ff7b into microsoft:master Nov 19, 2021
@dg0yt dg0yt deleted the netcdf-c branch November 19, 2021 08:32
@dg0yt dg0yt mentioned this pull request Dec 6, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] approved these changes

+1 more reviewer

@JackBoosY JackBoosY JackBoosY approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@JackBoosY JackBoosY

Labels

category:port-bug The issue is with a library, which is something the port should already support category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist category:port-update The issue is with a library, which is requesting update new revision info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dg0yt @JackBoosY @vicroms